-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Addinstance function #138
base: master
Are you sure you want to change the base?
Addinstance function #138
Conversation
Added test_add_instance.py
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #138 +/- ##
===========================================
+ Coverage 67.32% 78.23% +10.90%
===========================================
Files 14 15 +1
Lines 352 386 +34
===========================================
+ Hits 237 302 +65
+ Misses 115 84 -31
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
||
function_config: DLiteAddInstanceConfig | ||
|
||
def initialize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be even more useful to add the instance during the initialise phase?
Probably not... when thinking more about it.
"""Configuration for adding an instance to the collection.""" | ||
|
||
datamodel: str = Field( | ||
description="ID (URI or UUID) of the datamodel.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be mentioned that the data model must be available in dlite.storage_path
.
{ | ||
"7bad2efc-cb40-420b-aef9-2f909d038b46": { | ||
"meta": "http://onto-ns.com/meta/0.1/Result", | ||
"dimensions": { | ||
"natoms": 2, | ||
"ncoords": 3 | ||
}, | ||
"properties": { | ||
"potential_energy": 0, | ||
"forces": [[0, 0, 0], [0, 0, 0]] | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no point in committing a file that is generated.
But more worrying is that the properties are uninitialised. We should test that they are correctly assigned.
) | ||
|
||
pipeline = add_instance >> generate | ||
pipeline.get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[After running the pipeline, we should test that the file is created and has the expected content. For example
import numpy as np
inst = dlite.Instance.from_location("json", f"{outdir}/test_add_instance.json", options="mode=r")
assert np.allclose(inst.potential_energy, 3.2e-19)
assert np.allclose(inst.forses, [[1.2, 2.3, 3.4], [0.2, 3.4, 4.5]])
…ite into addinstance_function
Description:
Added function for creating instance
Type of change:
Checklist for the reviewer:
This checklist should be used as a help for the reviewer.